Repository navigation
Fix #15159 Wrong varid: function declaration with pointer-to-array parameter of unknown type - #8981
wjakobsson wants to merge 2 commits into
Conversation
|
Test results for commit 56694a0 (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8981/ Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong. |
dmcppcheck
left a comment
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
The PR introduces a varid regression. The condition now treats any name(...)[ in the first argument as a declaration. Because of that, global variables initialised with a subscripted function call, such as int x(g(1)[0]);, no longer get a varid. Base handled them correctly. The check should only accept the declarator shape (T (*p)[N] or T (&r)[N]), not every (...)[. The new test case passes and the existing varid tests still pass.
Testing
I built cppcheck and testrunner in pr/ and cppcheck in base/. ./testrunner TestVarID reports 0 failed tests out of 227.
Regression: scratch/a.cpp:
int* g(int);
int x(g(1)[0]);
int y(g(1)[0] + 1);
A h(T (*p)[16]);
A k(T (*p)[16], int n);
void f() { x = 1; y = 2; }Command: cppcheck --debug --verbose a.cpp
base:
2: int x@var1 ( g ( 1 ) [ 0 ] ) ;
3: int y@var2 ( g ( 1 ) [ 0 ] + 1 ) ;
4: A h@var3 ( T ( * p@var4 ) [ 16 ] ) ;
6: void f ( ) { x@var1 = 1 ; y@var2 = 2 ; }
pr:
2: int x ( g ( 1 ) [ 0 ] ) ;
3: int y ( g ( 1 ) [ 0 ] + 1 ) ;
4: A h ( T ( * p@var1 ) [ 16 ] ) ;
6: void f ( ) { x@exprUNIQUE = 1 ; y@exprUNIQUE = 2 ; }
The PR fixes h and k. However, the global variables x and y lose their varids, and their later uses become unrelated expressions. I did not find a warning that changes because of this; I only compared the varid dumps.
Automated review of 56694a0 by Claude. The pull request was built and tested locally.
| if (tok3->isLiteral() || | ||
| (tok3->isName() && (variableMap.hasVariable(tok3->str()) || | ||
| (tok3->strAt(-1) == "(" && Token::simpleMatch(tok3->next(), "(") && !Token::simpleMatch(tok3->linkAt(1)->next(), "(")))) || | ||
| (tok3->strAt(-1) == "(" && Token::simpleMatch(tok3->next(), "(") && !Token::Match(tok3->linkAt(1)->next(), "(|[")))) || |
There was a problem hiding this comment.
AI review
The new (|[ check is too broad. Any name(...)[ as the first argument now stops the code from being treated as a variable initialisation. So int x(g(1)[0]); at global scope loses its varid, and so does int y(g(1)[0] + 1);. Base gave them x@var1 and y@var2.
Only the declarator form should be excluded, for example by also requiring Token::Match(tok3->next(), "( *|&|&& %name% ) ["). Even with that, g(*p)[0] is ambiguous, but the variableMap.hasVariable check already handles a known p. A test case like int x(g(1)[0]); should be added next to the new one.
No description provided.